Skip to content

Fix mock client strategy #92

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 1, 2017
Merged

Conversation

captn3m0
Copy link
Contributor

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Related tickets -
Documentation -
License MIT

What's in this PR?

  • The Strategy returned a invalid array earlier.

The return value is always an array with zero or more elements.
Each element is an array with two keys
['class' => string, 'condition' => mixed]

The older value would lead to Illegal string offset 'class' error:

Illegal string offset 'class'

/vendor/php-http/discovery/src/ClassDiscovery.php:70
/vendor/php-http/discovery/src/HttpClientDiscovery.php:25
/src/php/Client.php:22
/tests/ClientTest.php:18

Why?

Mock Client Stategy Discovery is broken without this fix.

Checklist

  • Updated CHANGELOG.md to describe BC breaks / deprecations | new feature | bugfix
  • Documentation pull request created (if not simply a bugfix)

To Do

  • If the PR is not complete but you want to discuss the approach, list what remains to be done here

I'm not used to phpspec, so I'm not sure if the spec suffices here, or what can be done to improve it. I attempted using HttpClientDiscovery with the strategy set to mock, but that required adding too many dependencies.

- The Strategy returned a invalid array earlier.

  >The return value is always an array with zero or more elements.
  >Each  element is an array with two keys
  >['class' => string, 'condition' => mixed]
- Added entry in CHANGELOG as well
@sagikazarmark
Copy link
Member

LGTM

@dbu dbu merged commit 3d11c66 into php-http:master Mar 1, 2017
@dbu
Copy link
Contributor

dbu commented Mar 1, 2017

thank you!

@captn3m0 captn3m0 deleted the fix_mock_strategy branch March 1, 2017 12:41
@captn3m0
Copy link
Contributor Author

captn3m0 commented Mar 1, 2017

Any chance this can get a minor version bump?

@Nyholm
Copy link
Member

Nyholm commented Mar 1, 2017

No, but you should get a patch release. I will do that tonight unless someone else beat me to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants